[#1936] Migrate c-segment.vue to typescript #2035

jq1836 · 2023-09-13T14:48:56Z

Part of #1936

Proposed commit message

Currently, there is still some JavaScript code which remains 
unmigrated. This allows for type unsafe code to be written, 
potentially resulting in unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript 
code to facilitate future changes to the code.

Other information

Changes made

Converted JavaScript code in c-segment.vue to TypeScript.
Modified potentially dangerous code (see below).

Dangerous code

The following code makes use of the behavior of the || operator where the result will be the first operand given that the first operand is not false or null or any value that is represented by the value 0. In this case, this.segment.knownAuthor has been declared to be string or null in segment.ts. The consequence of this would be that the result, given that the first operand is a string that is not the empty string, '', would be the string itself. Due to common naming conventions, one would expect isOpen to be of type boolean, however, in actuality, it would be a string, hence the potentially dangerous behavior which currently works as intended, but may not in the future with more features relating to segment being added.

The same can be said for canOpen, however, it is not as dangerous due to the && operator taking into consideration both operands and returning a boolean value only. But in order to prevent future misunderstandings where someone mistakenly assumes that this.segment.knownAuthor is a boolean value from its use in the canOpen data field, I have opted to disambiguate this.

data() {
    return {
      isOpen: this.segment.knownAuthor || this.segment.lines.length < 5,
      canOpen: this.segment.knownAuthor && this.segment.lines.length > 4,
      transparencyValue: '30' as string,
    };
},

ckcherry23

Changes LGTM! @jq1836

ckcherry23 · 2023-09-19T04:49:41Z

frontend/src/components/c-segment.vue

+      isOpen: (this.segment.knownAuthor !== null) || this.segment.lines.length < 5 as boolean,
+      canOpen: (this.segment.knownAuthor === null) && this.segment.lines.length > 4 as boolean,


Thanks for fixing this part of the code with null checks!

vvidday

LGTM, thanks for providing the detailed explanation regarding the changes

github-actions · 2023-09-30T12:22:35Z

The following links are for previewing this pull request:

Dashboard Preview: https://dashboard-2035-pr-reposense-reposense.surge.sh
Docs Preview: https://docs-2035-pr-reposense-reposense.surge.sh

* [#2027] Fix date range bug (#2034) Currently, users are unable to select a zoom range that includes the until date. This results in misleading data being presented to users. * [#2039] Update cypress minimum requirement to 12.15.0 (#2041) Chrome bug is causing cypress to fail to open a browser on Github Actions, causing frontend tests and CI to fail. Upgrading cypress to greater than 12.15.0 will fix this issue. Let's upgrade cypress to fix the failing CI. * [#1936] Migrate c-segment.vue to typescript (#2035) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate load-font-awesome-icons.js to typescript (#2040) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#2045] Fix cypress zoom feature test (#2047) Currently, Cypress zoom feature tests are failing due to a recent change in behavior caused by a bug fix. With the tests failing, we are unable to detect any future regressions. Let's update the Cypress tests to test for the new intended behavior. * [#1936] Migrate random-color-gen.js to typescript (#2043) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate random-color-generator.js JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate c-segment-collection.vue to typescript (#2036) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate c-resizer.vue to typescript (#2038) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * Bump zod from 3.20.6 to 3.22.3 in /frontend (#2048) Bumps [zod](https://github.com/colinhacks/zod) from 3.20.6 to 3.22.3. - [Release notes](https://github.com/colinhacks/zod/releases) - [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md) - [Commits](colinhacks/zod@v3.20.6...v3.22.3) --- updated-dependencies: - dependency-name: zod dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @cypress/request and cypress in /frontend/cypress (#2042) Bumps [@cypress/request](https://github.com/cypress-io/request) to 3.0.1 and updates ancestor dependency [cypress](https://github.com/cypress-io/cypress). These dependencies need to be updated together. Updates `@cypress/request` from 2.88.12 to 3.0.1 - [Release notes](https://github.com/cypress-io/request/releases) - [Changelog](https://github.com/cypress-io/request/blob/master/CHANGELOG.md) - [Commits](cypress-io/request@v2.88.12...v3.0.1) Updates `cypress` from 12.17.4 to 13.3.0 - [Release notes](https://github.com/cypress-io/cypress/releases) - [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md) - [Commits](cypress-io/cypress@v12.17.4...v13.3.0) --- updated-dependencies: - dependency-name: "@cypress/request" dependency-type: indirect - dependency-name: cypress dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [#1936] Migrate c-ramp.vue to typescript (#2037) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * Give partial credit if annotated author is not the same as the blame author * [#2054] Fix zoom view bug (#2055) Currently, when granularity is set to day or week, clicking on a ramp will open up a zoom view where commit messages are not being displayed and sorting by insertions does not result in any sorting. Let's fix the unintended behaviour of the zoom view. * [#1936] Migrate repo-sorter.js to typescript (#2052) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate repo-sorter.js to TypeScript code to facilitate future changes to the code. * [#1936] Migrate safari_date.js to typescript (#2053) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate safari_date.js to TypeScript code to facilitate future changes to the code. * Remove frontend JS lint (#2063) Currently, frontend linter is failing due to lint scripts checking javascript files, the last of which has been removed in PR #2053. Lets update the lint command to exclude javascript files front the check. * use full and partial credit color * [#1929] Add dynamic positioning support for tooltips (#2056) Currently, most tooltips are shown above buttons and text. When these tooltips appear at the top of the viewport, part of the tooltips will not be rendered. Let's implement changes such that these tooltips appear below the text or button, when appearing at the top of the viewport. * Add test cases for annotated author overriding last author's credit * revert merge from master * revert merge from master 58b7002 * [#1928] Fix tooltip zIndex such that it doesn't occlude next file title (#2057) Currently, if one hovers over a tooltip of the pinned title of a file whose content is scrolled almost completely, such that the title of the next file is just below the pinned title, the tooltip is not displayed appropriately, as the title of the next file obstructs it. Let's fix this issue. * [#1726] Update GitHub-specific references in codebase and docs (#2050) There are still leftover references specific to GitHub on parts of the codebase and docs that have been generalized to accept other remote git hosts. Let's update these GitHub references to use more general language. * Trigger workflow * Revert "Merge branch 'master' into 944-analyze-authorship" This reverts commit 950c912, reversing changes made to 4bd05a7. * fix frontend test failing --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: jq1836 <[email protected]> Co-authored-by: Chan Jun Da <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pratham Jain <[email protected]>

* [#2027] Fix date range bug (#2034) Currently, users are unable to select a zoom range that includes the until date. This results in misleading data being presented to users. * [#2039] Update cypress minimum requirement to 12.15.0 (#2041) Chrome bug is causing cypress to fail to open a browser on Github Actions, causing frontend tests and CI to fail. Upgrading cypress to greater than 12.15.0 will fix this issue. Let's upgrade cypress to fix the failing CI. * [#1936] Migrate c-segment.vue to typescript (#2035) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate load-font-awesome-icons.js to typescript (#2040) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#2045] Fix cypress zoom feature test (#2047) Currently, Cypress zoom feature tests are failing due to a recent change in behavior caused by a bug fix. With the tests failing, we are unable to detect any future regressions. Let's update the Cypress tests to test for the new intended behavior. * [#1936] Migrate random-color-gen.js to typescript (#2043) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate random-color-generator.js JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate c-segment-collection.vue to typescript (#2036) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * [#1936] Migrate c-resizer.vue to typescript (#2038) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * Bump zod from 3.20.6 to 3.22.3 in /frontend (#2048) Bumps [zod](https://github.com/colinhacks/zod) from 3.20.6 to 3.22.3. - [Release notes](https://github.com/colinhacks/zod/releases) - [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md) - [Commits](colinhacks/zod@v3.20.6...v3.22.3) --- updated-dependencies: - dependency-name: zod dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @cypress/request and cypress in /frontend/cypress (#2042) Bumps [@cypress/request](https://github.com/cypress-io/request) to 3.0.1 and updates ancestor dependency [cypress](https://github.com/cypress-io/cypress). These dependencies need to be updated together. Updates `@cypress/request` from 2.88.12 to 3.0.1 - [Release notes](https://github.com/cypress-io/request/releases) - [Changelog](https://github.com/cypress-io/request/blob/master/CHANGELOG.md) - [Commits](cypress-io/request@v2.88.12...v3.0.1) Updates `cypress` from 12.17.4 to 13.3.0 - [Release notes](https://github.com/cypress-io/cypress/releases) - [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md) - [Commits](cypress-io/cypress@v12.17.4...v13.3.0) --- updated-dependencies: - dependency-name: "@cypress/request" dependency-type: indirect - dependency-name: cypress dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [#1936] Migrate c-ramp.vue to typescript (#2037) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate the rest of the JavaScript code to TypeScript code to facilitate future changes to the code. * Give partial credit if annotated author is not the same as the blame author * [#2054] Fix zoom view bug (#2055) Currently, when granularity is set to day or week, clicking on a ramp will open up a zoom view where commit messages are not being displayed and sorting by insertions does not result in any sorting. Let's fix the unintended behaviour of the zoom view. * [#1936] Migrate repo-sorter.js to typescript (#2052) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate repo-sorter.js to TypeScript code to facilitate future changes to the code. * [#1936] Migrate safari_date.js to typescript (#2053) Currently, there is still some JavaScript code which remains unmigrated. This allows for type unsafe code to be written, potentially resulting in unintended behavior. Let's migrate safari_date.js to TypeScript code to facilitate future changes to the code. * Remove frontend JS lint (#2063) Currently, frontend linter is failing due to lint scripts checking javascript files, the last of which has been removed in PR #2053. Lets update the lint command to exclude javascript files front the check. * use full and partial credit color * [#1929] Add dynamic positioning support for tooltips (#2056) Currently, most tooltips are shown above buttons and text. When these tooltips appear at the top of the viewport, part of the tooltips will not be rendered. Let's implement changes such that these tooltips appear below the text or button, when appearing at the top of the viewport. * Add test cases for annotated author overriding last author's credit * revert merge from master * revert merge from master 58b7002 * [#1928] Fix tooltip zIndex such that it doesn't occlude next file title (#2057) Currently, if one hovers over a tooltip of the pinned title of a file whose content is scrolled almost completely, such that the title of the next file is just below the pinned title, the tooltip is not displayed appropriately, as the title of the next file obstructs it. Let's fix this issue. * [#1726] Update GitHub-specific references in codebase and docs (#2050) There are still leftover references specific to GitHub on parts of the codebase and docs that have been generalized to accept other remote git hosts. Let's update these GitHub references to use more general language. * Trigger workflow * Revert "Merge branch 'master' into 944-analyze-authorship" This reverts commit 950c912, reversing changes made to 4bd05a7. * fix frontend test failing * switch to originality score and threshold * update originality threshold * revert frontend code changes --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: jq1836 <[email protected]> Co-authored-by: Chan Jun Da <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pratham Jain <[email protected]>

jq1836 added 2 commits September 13, 2023 22:13

Convert c-segment to typescript

4e86dbf

Fix inverted isOpen and canOpen

20e5007

ckcherry23 self-requested a review September 19, 2023 04:46

ckcherry23 approved these changes Sep 19, 2023

View reviewed changes

ckcherry23 requested a review from a team September 19, 2023 04:53

chan-j-d added the s.ToReview label Sep 20, 2023

vvidday approved these changes Sep 23, 2023

View reviewed changes

vvidday added s.ToMerge and removed s.ToReview labels Sep 23, 2023

chan-j-d added 2 commits September 25, 2023 01:38

Merge branch 'master' into 1936-migrate-c-segment

8959bc0

Merge branch 'master' into 1936-migrate-c-segment

268ba1b

chan-j-d merged commit 46409aa into reposense:master Sep 30, 2023
10 checks passed

jq1836 deleted the 1936-migrate-c-segment branch September 30, 2023 12:41

chan-j-d mentioned this pull request Oct 28, 2023

Migrate entire frontend to TypeScript #1936

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#1936] Migrate c-segment.vue to typescript #2035

[#1936] Migrate c-segment.vue to typescript #2035

jq1836 commented Sep 13, 2023 •

edited by chan-j-d

Loading

ckcherry23 left a comment

ckcherry23 Sep 19, 2023

vvidday left a comment

github-actions bot commented Sep 30, 2023

		isOpen: (this.segment.knownAuthor !== null) \|\| this.segment.lines.length < 5 as boolean,
		canOpen: (this.segment.knownAuthor === null) && this.segment.lines.length > 4 as boolean,

[#1936] Migrate c-segment.vue to typescript #2035

[#1936] Migrate c-segment.vue to typescript #2035

Conversation

jq1836 commented Sep 13, 2023 • edited by chan-j-d Loading

Proposed commit message

Other information

Changes made

Dangerous code

ckcherry23 left a comment

Choose a reason for hiding this comment

ckcherry23 Sep 19, 2023

Choose a reason for hiding this comment

vvidday left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 30, 2023

jq1836 commented Sep 13, 2023 •

edited by chan-j-d

Loading